Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encourage the use of root_path in production to ensure single deployment #1712

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

lennartkats-db
Copy link
Contributor

Changes

This updates mode: production to allow root_path to indicate uniqueness. Historically, we required run_as for this, which isn't actually very effective for that purpose. run_as also had the problem that it doesn't work for pipelines.

This is a cherry-pick from #1387

@lennartkats-db lennartkats-db requested a review from pietern August 28, 2024 07:43
bundle/config/mutator/cleanup_targets.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode_test.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Show resolved Hide resolved
bundle/config/mutator/cleanup_targets.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Show resolved Hide resolved
bundle/config/mutator/select_target.go Show resolved Hide resolved
@lennartkats-db lennartkats-db force-pushed the cp-encourage-root-path branch 2 times, most recently from 742210c to 3973c50 Compare October 12, 2024 10:01
@lennartkats-db
Copy link
Contributor Author

@pietern @andrewnester could you take another look at this PR? The remaining thread should be resolved: #1712 (comment)

bundle/config/bundle.go Outdated Show resolved Hide resolved
bundle/config/mutator/process_target_mode.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Dec 9, 2024

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1712
  • Commit SHA: 293d7808a66d282d90901ef3e837f20c75656580

Checks will be approved automatically on success.

// report an error for them.
return diag.Warningf("target with 'mode: production' should " + advice)
}
return diag.Errorf("target with 'mode: production' must " + advice)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For follow-up: the advice should be a multi-line string in the Detail field to render nicely. As is, it is shown as a single (wrapped) line. The warning case will light up in all cases where mode: production is used and the root path is not specified.

Also, this doc page should be updated https://docs.databricks.com/en/dev-tools/bundles/deployment-modes.html#production-mode as well and linked from the warning to explain why the warning is shown while users haven't changed anything.

// and neither is setting a principal.
// We only show a warning for these cases since we didn't historically
// report an error for them.
return diag.Warningf("target with 'mode: production' should " + advice)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Encourage" in the title means it's more a recommendation than a warning.

Since it will light up for existing users, a "recommendation" here is less forceful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants